Add support for Symfony Mailer #1

ghost · 2020-08-11T18:07:48Z

Hi @ThomasLandauer , I've been looking at your PR Codeception#9 , and I've some changes that I want to suggest to you.

First, I used an optional parameter in the seeEmailIsSent function because it is a way to add this new functionality without creating BC, by default it is 'swiftmailer' as before, but if the user so wishes, he can pass 'symfony_mailer' as the second parameter to this function to use that 'Profile Collector'.

-    public function seeEmailIsSent($expectedCount = null)
+    public function seeEmailIsSent($expectedCount = null, $mailer = 'swiftmailer')

Secondly, I validated that at least one of the 'Profile Collector' exists:

+        switch ($mailer) {
+            case 'swiftmailer':
                if (!$profile->hasCollector('swiftmailer')) {
                    $this->fail(
                         'Emails can\'t be tested without SwiftMailer connector.
+                        If you are using Symfony Mailer use "symfony_mailer" as second parameter'
                    );
                }
+                break;
+            case 'symfony_mailer':
+                if (!$profile->hasCollector('mailer')) {
+                    $this->fail('Emails can\'t be tested without Symfony Mailer connector');
+                }
+                break;
+            default:
+                $this->fail('Invalid mailer argument. Allowed Options: "swiftmailer" or "symfony_mailer"');
+        }

Then, I added this logic, in this case it's enough to write 'else' and not 'elseif' since above we verified that there is a valid 'Profile Collector'.

+        if ($mailer === 'swiftmailer') {
            $realCount = $profile->getCollector('swiftmailer')->getMessageCount();
+        } else {
-           $realCount += count($profile->getCollector('mailer')->getEvents()->getMessages());
+           $realCount = count($profile->getCollector('mailer')->getEvents()->getMessages());
+        }

And finally I took the opportunity to add support for Symfony Mailer in the debugResponse function, in this way the complete module is now compatible with Symfony Mailer without breaking the code of the users who use it with Swift Mailer. (So the name of the PR should change).

            if ($profile->hasCollector('swiftmailer')) {
                $messages = $profile->getCollector('swiftmailer')->getMessageCount();
                if ($messages) {
                    $this->debugSection('Emails', $messages . ' sent');
                }
+            } elseif ($profile->hasCollector('mailer')) {
+                $messages = count($profile->getCollector('mailer')->getEvents()->getMessages());
+                if ($messages) {
+                    $this->debugSection('Emails', $messages . ' sent');
+                }
             }

I'll stay tuned to your comments. Cheers!

Adding `const`s for #1

ThomasLandauer · 2020-08-12T12:09:45Z

Yeah, I have to admit that I didn't take care about SwiftMailer ;-) So this is certainly a good idea!

The problem I'm seeing is this: If we now introduce $I->seeEmailIsSent(1, 'symfony_mailer'); how can we ever get rid of the second argument again? We'd need to change the default value, which is a BC break, and will take, well, forever...

So I have another idea: Put it into the config file functional.suite.yml! Something like:

modules:
    enabled:
        - Symfony:
            mailer: 'swiftmailer' # or 'symfony_mailer'

Advantages:

Set it once, then forget about it
I doubt that anybody uses both mailers in parallel. But even this would be possible, since you can change the configuration on the fly with $this->getModule('Symfony')->_reconfigure(['mailer' => 'swiftmailer']);

What do you think?

ghost · 2020-08-12T14:42:03Z

Even better 👍
now I just have to see how I write that 🤣 , but it looks interesting
i will try to do it that way, thanks you

ghost · 2020-08-12T15:32:36Z

Ok, i think i got it. 😃

modules:
    enabled:
        - Symfony:
            mailer: 'symfony_mailer'

and

        //...
        $I->seeEmailIsSent(2);

results in:

Time: 00:12.905, Memory: 24.00 MB

OK (1 tests, 1 assertions)

ThomasLandauer · 2020-08-12T18:00:02Z

I'm merging this, but I didn't double-check it :-)

ThomasLandauer · 2020-08-26T13:37:34Z

@TavoNiievez looks like there are some problems - could you take a look? https://github.com/Codeception/module-symfony/actions/runs/205934965

ghost · 2020-08-26T14:12:13Z

@ThomasLandauer I saw them a couple of weeks ago when you merged, but it seems to be a dependency related issue installing the project (friendsofphp/php-cs-fixer 3 with symfony/console 5 to be exact) and not really with the code I edited. Resolving those conflicts should be a separate PR.

ThomasLandauer · 2020-08-27T12:05:47Z

OK, I wasn't sure if you'd get the notification too.

BTW: I think the main problem is that the Symfony module currently doesn't have a maintainer. Would you be interested?

ghost · 2020-08-27T22:45:01Z

For me it would be a pleasure to help.

I just joined as a Specify and Verify maintainer and I would like to implement some changes in Module Asserts and Lib Asserts,

but after that I was planning to send PR's to the Symfony Module and the Doctrine Module.

It's just that I think it's essential to do the work first in those libraries that are smaller and then make changes in a slightly more complex one like the Symfony module.

so my answer is: 'Eventually yes' 😃

ThomasLandauer · 2020-08-27T23:09:31Z

@DavertMik, @Naktibalda are you still looking for a maintainer of the Symfony module? Codeception/Codeception#4243 (comment) May I suggest TavoNiievez?

* Update Symfony.php Fixing `seeEmailIsSent()` for [Symfony Mailer](https://symfony.com/doc/master/mailer.html) How did I figure it out (=Symfony internals): [`mailer_debug.php`](https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Resources/config/mailer_debug.php) sends the [`MessageDataCollector`](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Mailer/DataCollector/MessageDataCollector.php) to [`mailer.html.twig`](https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/mailer.html.twig#L9), which shows that the number of `events.messages` is the thing to look for. I'm just adding this number to the number returned by the SwiftMailer Collector. Limitations (=TODO): * I have both SwiftMailer and Symfony Mailer installed - don't know what happens if you only have one (or none) * I didn't add any tests - don't know how to do that * Update Symfony.php * Added a more descriptive fail comment * Update Symfony.php Adding `const`s for ThomasLandauer#1 * mailer setting in module config * Fix little Typo Co-authored-by: Gustavo Nieves <37160403+TavoNiievez@users.noreply.github.com>

Update Symfony.php

a295628

ghost changed the title ~~Update Symfony.php~~ Add support for Symfony Mailer Aug 11, 2020

Added a more descriptive fail comment

2f84671

ghost mentioned this pull request Aug 11, 2020

Update Symfony.php Codeception/module-symfony#9

Merged

Update Symfony.php

dfaf6f0

Adding `const`s for #1

Gustavo Nieves added 2 commits August 12, 2020 10:27

mailer setting in module config

80b7c32

Fix little Typo

70d5c65

ThomasLandauer merged commit 70042e1 into ThomasLandauer:patch-1 Aug 12, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add support for Symfony Mailer #1

Add support for Symfony Mailer #1

Uh oh!

ghost commented Aug 11, 2020 •

edited by ghost

Loading

Uh oh!

ThomasLandauer commented Aug 12, 2020

Uh oh!

ghost commented Aug 12, 2020

Uh oh!

ghost commented Aug 12, 2020 •

edited by ghost

Loading

Uh oh!

ThomasLandauer commented Aug 12, 2020

Uh oh!

ThomasLandauer commented Aug 26, 2020

Uh oh!

ghost commented Aug 26, 2020

Uh oh!

ThomasLandauer commented Aug 27, 2020

Uh oh!

ghost commented Aug 27, 2020

Uh oh!

ThomasLandauer commented Aug 27, 2020

Uh oh!

Uh oh!

Add support for Symfony Mailer #1

Add support for Symfony Mailer #1

Uh oh!

Conversation

ghost commented Aug 11, 2020 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ThomasLandauer commented Aug 12, 2020

Uh oh!

ghost commented Aug 12, 2020

Uh oh!

ghost commented Aug 12, 2020 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ThomasLandauer commented Aug 12, 2020

Uh oh!

ThomasLandauer commented Aug 26, 2020

Uh oh!

ghost commented Aug 26, 2020

Uh oh!

ThomasLandauer commented Aug 27, 2020

Uh oh!

ghost commented Aug 27, 2020

Uh oh!

ThomasLandauer commented Aug 27, 2020

Uh oh!

Uh oh!

ghost commented Aug 11, 2020 •

edited by ghost

Loading

ghost commented Aug 12, 2020 •

edited by ghost

Loading